fix(clients): make POST body related to the endpoint, assert body in tests #849
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for api-clients-automation canceled.
|
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
millotp
previously approved these changes
Jul 18, 2022
Doing more tests on the |
shortcuts
force-pushed
the
fix/clients-empty-body-request
branch
from
9b758b5
to
eca2fc5
Compare
shortcuts
changed the title
DELETE requests
millotp
reviewed
Jul 19, 2022
Yup I'm importing the echoRequester fixes from the other PR, just making sure it works properly |
shortcuts
force-pushed
the
fix/clients-empty-body-request
branch
from
81e42dc
to
2903221
Compare
shortcuts
changed the title
DELETE requests
shortcuts
force-pushed
the
fix/clients-empty-body-request
branch
from
94464cc
to
bfc99ff
Compare
shortcuts
changed the title
millotp
reviewed
Jul 20, 2022
...oliasearch-client-java-2/algoliasearch-core/src/main/java/com/algolia/utils/JSONBuilder.java
Outdated
Show resolved
Hide resolved
.../algoliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts
Outdated
Show resolved
Hide resolved
| @@ -50,7 +50,8 @@ private function normalize($options) | |||
| 'headers' => [ | |||
| 'x-algolia-application-id' => $this->config->getAppId(), | |||
| 'x-algolia-api-key' => $this->config->getAlgoliaApiKey(), | |||
| 'User-Agent' => $this->config->getAlgoliaAgent() !== null | |||
| 'User-Agent' => | |||
There was a problem hiding this comment.
I think the php changes can be moved to another PR until @damcou is back
There was a problem hiding this comment.
The changes for the PHP client here still fixes the POST methods that were not working previously (e.g. browse). However, I've removed the transporter/CTS changes in the meantime
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🧭 What and Why
🎟 JIRA Ticket: https://algolia.atlassian.net/browse/DI-321
The issue was discovered in #823, but I've moved the fix for the clients here.
Changes included:
The goal of this PR is to assert the body of each requests in the CTS even when we don't expect to have one.
On some
POSTrequests, the body can be optional (e.g.browse), but needs to be a valid JSON object, when no body is expected at all (e.g.clearAllSynonyms), an empty string is considered as valid.In any case, an empty object (Some endpoints do not expect a body at all, so we need to move that logic at the method level since it can't be factorized.'{}') is considered as valid.Issue
browsemethod with only anindexNamethrows an empty body errorclearRulesmethod with a body throws an unexpected body errorNext
Fix
nullbody forGETandDELETEmethods on the PHP client, once @damcou is back: https://algolia.atlassian.net/browse/APIC-591🧪 Test
deleteApiKeyfor theDELETEmethod changebrowsewith empty object only for thePOSTmethod change